-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[core.logging] Deprecates legacy logging dest, json, verbosity and rotate configurations #94238
[core.logging] Deprecates legacy logging dest, json, verbosity and rotate configurations #94238
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TinaHeiligers One thing that occurred to me when summarizing these changes for the beats team is that we might want to mention in the docs that switching to the new logging config is going to mean you start seeing duplicate log entries in both formats.
Most users will have not run into this yet, but if they are ingesting their Kibana logs somewhere, it would be good for them to know that using the new config will result in additional log entries which are in a different structure.
9639539
to
abae294
Compare
…et legacy logging level when not opted in to KP logging
Pinging @elastic/kibana-core (Team:Core) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question I want to raise is whether we should consider adding deprecation notices for the undocumented logging.filter
, logging.events.log
, and logging.events.error
... AFICT those are the only remaining legacy logging configs that would not be deprecated once this PR is merged.
On the one hand, as they were never officially documented, there should be no expectation of them not breaking. But on the other hand, if someone stumbled upon these settings, it would make their lives easier to get a deprecation notice warning them when they are in use. WDYT?
) { | ||
log( | ||
'"logging.rotate" and sub-options have been deprecated and will be removed in 8.0. ' + | ||
'Moving forward, you can enabled log rotation using the "rolling-file" appender for a context ' + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enabled
-> enable
is a typo
context
-> logger
is a nit (In #90764 we switched from labeling loggers with context
to just using name
... but there are probably a few other places in the logging docs where we'd need to update this, so no strong feelings on it).
'Moving forward, you can enabled log rotation using the "rolling-file" appender for a context ' + | |
'Moving forward, you can enable log rotation using the "rolling-file" appender for a logger ' + |
@lukeelmers This is a tough call. I'm not apposed to helping folks out if we have an alternative in the KP logging system. We don't yet support filtering in the KP so I'd err on the side of not adding a deprecation warning for that. We could add deprecation warnings for However, I'm not sure if it would be more of a surprise for folks to start seeing these warnings only to have the settings removed completely in a few minors. I'll add them and we can see what other reviewers think. |
I prefer we show warnings for all settings that will no longer work, even if they were never documented and/or don't have a replacement in 8.0. +1 on the warnings for all 3 of those configs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really close! The only scenario that doesn't seem to be working quite right for me is the one where I have the KP logger configured (logging.root.appenders
is set) and I'm using the --verbose
flag. It's still showing a warning log message about logging.verbose
. I left a comment below about how to fix the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updates here LGTM, thanks for adding those logging.events
and logging.filter
deprecations too!
if (has(settings, 'logging.events.error')) { | ||
log( | ||
'"logging.events.error" has been deprecated and will be removed ' + | ||
'in 8.0. Moving forward, you can use "logging.root.level: error" in your logging configuration. ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: Could still link to the docs for both of the logging.events
deprecations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lukeelmers is it ok if we do that once the docs for the KP logging system are added and I go back and change the other links?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for me 🙂
@elasticmachine merge upstream |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
💔 Backport failed❌ 7.x: Commit could not be cherrypicked due to conflicts To backport manually, check out the target branch and run: |
…tate configurations (elastic#94238) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # docs/migration/migrate_8_0.asciidoc # src/core/server/config/deprecation/core_deprecations.ts
Summary
Resolves #82431.
Closes #86125
This PR deprecates legacy logging configuration in favor of the new KP logging system.
These include:
logging.silent
logging.quiet
logging.verbose
logging.dest
logging.json
logging.rotate
& sub-options:enabled
,usePolling
,pollingInterval
,everyBytes
,keepFiles
logging.events
logging.filter
The deprecation warnings shown when legacy logging is configured include a short description about KP logging config alternatives and link to the KP logging README. These links will be changed to reference the user-facing docs once #57502 is addressed.
The deprecations also apply to any configuration passed in as cli arguments except for the short-hand versions of
--verbose
and--silent
logging levels.--verbose
is now handled slightly differently and will do the following:env === 'development
), deprecation warnings for using--json
won't be shown (we explicitly set--logging.json
in dev cli).all
logging.verbose = true
)Release Notes
Deprecates legacy logging configuration in favor of the new Kibana Platform logging system. For example, deprecates
logging.json
andlogging.rotate.*
.--verbose
will replace legacy-format logs with Kibana platform log format iflogging.root.appenders
is configured and won't show a deprecation warning. If Kibana platform logging is not configured,--verbose
setslogging.verbose: true
and will warn of the deprecated configuration.Checklist
Delete any items that are not applicable to this PR.
For maintainers